Skip to content

passthrough example for ext fs#96

Closed
kuchling wants to merge 2 commits intolibfuse:mainfrom
kuchling:ext_pass
Closed

passthrough example for ext fs#96
kuchling wants to merge 2 commits intolibfuse:mainfrom
kuchling:ext_pass

Conversation

@kuchling
Copy link
Copy Markdown

@kuchling kuchling commented Nov 23, 2023 •
edited
Loading

Ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git. I don't know how good this testsuit is, but i also found no better or new one.
Could be merged, i have only tested with ext4 as source fs. This testsuit has a lot of testcases.

  • create syscal didn't set mode and ownership
  • chown syscal with no change of uid gid should mofify ctime on ext fs (seems to be different on other filesystems)
  • allow_other, dev, suid was set within options to run all testcases

@Nikratio
Copy link
Copy Markdown
Contributor

Nikratio commented Nov 24, 2023

Thanks! Could you elaborate on the differences to the existing passthroughfs.py? Might there be a way to merge the changes in there instead?

@kuchling
Copy link
Copy Markdown
Author

kuchling commented Nov 27, 2023

Thanks! Could you elaborate on the differences to the existing passthroughfs.py? Might there be a way to merge the changes in there instead?

Ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git. I don't know how good this testsuit is, but i also found no better or new one.
Could be merged, i have only tested with ext4 as source fs. This testsuit has a lot of testcases.

create syscal didn't set mode and ownership
chown syscal with no change of uid gid should mofify ctime on ext fs (seems to be different on other filesystems)
allow_other, dev, suid was set within options to run all testcases

@kuchling kuchling closed this Nov 27, 2023
@kuchling kuchling reopened this Nov 27, 2023
@Nikratio
Copy link
Copy Markdown
Contributor

Nikratio commented Nov 28, 2023

Thanks! Could you elaborate on the differences to the existing passthroughfs.py? Might there be a way to merge the changes in there instead?

Ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git.

To rephrase, the difference between the existing passthroughfs.py and your ext_passthroughfs.py is that the latter passes the fstest, and the former does not?

If so, what would it take to modify passthroughfs.py to pass the test? Why do we need a new filesystem?

@kuchling
Copy link
Copy Markdown
Author

kuchling commented Nov 29, 2023

I do not need a new fs, it was choosen to not break other stuff and because different lower fs have different behavoir see second change, regarding time. The only relevant change for passthroughps.py in general in my opinion is to correct the create syscall as stated to fix owner and mode.
Why the testsuite? It was to get maximum compatiblility with the original fs, means if passthroughfs.py will be used to intercept some things (every ones one fun and idea) it should behave as good as possible to the original fs.

@Nikratio
Copy link
Copy Markdown
Contributor

Nikratio commented Nov 29, 2023

I do not need a new fs, it was choosen to not break other stuff and because different lower fs have different behavoir see second change, regarding time.

I do not see any changes, I just see one large new file being added.

Could you please modify passthroughfs.py instead of adding a new file?

ThomasWaldmann
ThomasWaldmann requested changes Dec 1, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the updated PR, some feedback from me.

Comment thread examples/passthroughfs.py
Comment on lines +316 to +322
if fields.update_mode is False and \
fields.update_uid is False and \
fields.update_gid is False:
if chown == os.chown:
chown(path_or_fh, -1, -1, follow_symlinks=False)
if chown == os.fchown:
chown(path_or_fh, -1, -1)
Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann Dec 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is rather strange:

  • why is chown called at all if neither uid nor gid shall be set (both are -1)?
  • is the x is False really wanted / correct instead of the usual boolean evaluation using not x? consider x == 0 or x == None, for example.
  • in 311/314 chown is always called with follow_symlinks=False, no matter whether path or fh is used, so 320/322 look inconsistent with these.

Comment thread examples/passthroughfs.py

if __name__ == '__main__':
main()

Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann Dec 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accidental change?

Comment thread examples/passthroughfs.py
@@ -1,5 +1,6 @@
#!/usr/bin/env python3
'''
ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git
Copy link
Copy Markdown
Collaborator

@ThomasWaldmann ThomasWaldmann Dec 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather a remark or property of this implementation, but nothing the description of this should start with, so move it a bit lower, please.

@Nikratio
Copy link
Copy Markdown
Contributor

Nikratio commented Dec 28, 2025

I'm closing this for now, since there's been no update for 2 years.

@Nikratio Nikratio closed this Dec 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ThomasWaldmann ThomasWaldmann ThomasWaldmann requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kuchling @Nikratio @ThomasWaldmann